Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge changes from Mr-Dave Master #29

Closed
wants to merge 44 commits into from

Conversation

Mr-Dave
Copy link

@Mr-Dave Mr-Dave commented Aug 31, 2014

This is obviously a massive pull. Various bugs and associated fixes were implemented throughout the development process of my fork and illustrated in the changes below. It may be easier to review the end result rather than the step by step evolution. As far as I can determine, this pull would resolve all the outstanding rtsp issues listed in your fork.

Dave and others added 30 commits June 13, 2014 19:52
config.h is auto generated upon configure, motion.mk808.conf was temporary experimental config from test branch - it's not needed in the scope of current fork
CHANGELOG INSTALL ffmpeg.c ffmpeg.h netcam.c netcam_rtsp.c
Remove some files that should not be in source tree, minor changes
Conflicts:
	configure
	netcam.c
	netcam_rtsp.c
Merge with latest "unstable" tree of Mr-Dave
@tosiara
Copy link

tosiara commented Aug 31, 2014

I also vote for this pull. Tested it a lot, have been running stable for long time

@aklomp
Copy link

aklomp commented Sep 5, 2014

Congratulations on shipping this massive amount of work, but 048791e appears to include my Pull #20 with my commit message squashed.

@Mr-Dave
Copy link
Author

Mr-Dave commented Sep 5, 2014

All changes were not properly attributed.

@Mr-Dave Mr-Dave closed this Sep 5, 2014
@aklomp
Copy link

aklomp commented Sep 5, 2014

No need to close this pull request and lose all the comments, you can force-push over the existing pull request (your master branch), and this request will update automatically.

@tosiara
Copy link

tosiara commented Sep 8, 2014

What was the purpose of all those rebase, revert and recommit? Now it just a little more mess in repo. The fork was ready to auto merge without any conflicts. @aklomp 's commit has now been removed from @Mr-Dave 's fork - why? I'm really confused now

@aklomp
Copy link

aklomp commented Sep 8, 2014

About the rebase: I was noting that this pull request integrated a number of other pull requests without merging them in properly. Although the codebase gets the improvements, it removes the attribution, commit messages, and history associated with those pulls. It also obscures which pull requests are dependent on each other, making it it harder for the maintainer to decide on an individual basis which to apply, and in which order. I suggested a few methods to resolve this in the comments of Issue #20. Best thing, I think, would be for the maintainer to merge the issues one by one in order of dependence.

I have no strong feelings about this, my point was to make a housekeeping comment. It was not my intention to get my work removed from Mr-Dave's tree, but rather to have it merged properly.

I applaud the massive work Mr-Dave has done (and, importantly, shipped), so feel free on my part to reopen this pull request as it was.

@Mr-Dave
Copy link
Author

Mr-Dave commented Sep 8, 2014

Aklomp indicated that my fork and associated merge did not provide
proper attribution of his work. In order to make sure that Aklomp gets
proper attribution for his work, I rebased the files affected by his
work within my fork. Therefore, if my fork is merged into the master,
there will not be any conflicts with his pull request. As an
alternative, he can issue a pull request to my fork and if my fork is
merged, it would include his attribution that way.

Dave

On 9/8/2014 2:13 AM, tosiara wrote:

What was the purpose of all those rebase, revert and recommit? Now it
just a little more mess in repo. The fork was ready to auto merge
without any conflicts. @aklomp https://github.com/aklomp 's commit
has now been removed from @Mr-Dave https://github.com/Mr-Dave 's
fork - why? I'm really confused now


Reply to this email directly or view it on GitHub
#29 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants